-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merge develop into main for v0.4.0 release #49
Conversation
…in-test Fix maximum concentration in positive electrode in test file
… Electrode, renamed Chemistry field to Particle
… sto limits if basic checks are not successful)
…ion-based-on-models
…dels Validation based on models
Add support for well-mixed, blended electrodes that contain more than one active material
Add parametrisation examples
…params Issue 26 user defined params
…ng for tolerances
Fix one typo and description of conductivity in solid
Validate the sto limits subbed into the OCPs give the correct voltage limits
bump version number
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a lot of changes to review! :) See a couple of minor comments I left... Otherwise looks good.
README.md
Outdated
```python | ||
print(bpx.BPX.schema_json(indent=2)) | ||
``` | ||
|
||
According to the `pydantic` docs, the generated schemas are compliant with the specifications: JSON Schema Core, JSON Schema Validation and OpenAPI. | ||
|
||
## 📖 Documentation | ||
API documentation for the `bpx` package can be built locally using [Sphinx](https://www.sphinx-doc.org/en/master/). To build the documentation first [clone the repository](https://github.com/git-guides/git-clone), then run the following command: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This didn't work straight away for me ('No module named bpx' error). I just git-cloned the repo (dev branch) and tried the command below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you have to install bpx first, will update the instructions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I realised that I had installed the previous version of BPX, that's why it probably complained.
CHANGELOG.md
Outdated
# [v0.4.0](https://github.com/FaradayInstitution/BPX/releases/tag/v0.4.0) | ||
|
||
- Added five parametrisation examples (two DFN parametrisation examples from About:Energy open-source release, blended electrode definition, user-defined 0th-order hysteresis, and SPM parametrisation). ([#45](https://github.com/FaradayInstitution/BPX/pull/45)) | ||
- Allow user-defined parameters to be added using the field ["Parameterisation"]["User-defined"] ([#44](https://github.com/pybamm-team/BPX/pull/44)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are still a few links to the old (pybamm-team) repo here and below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, will update
@ikorotkin thanks, all these things should have already been reviewed once when they were merged into develop, so thanks for the careful re-review! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's only the link to pybamm-team/bpx-example
should be fixed, and we can merge it.
README.md
Outdated
@@ -8,7 +8,7 @@ This repository features a Pydantic-based parser for JSON files in the BPX forma | |||
|
|||
To support the new open standard, [About:Energy](https://www.aboutenergy.io/) have supplied two parameters sets for an NMC and LFP cell. The BPX files and associated examples and information can be found on the [A:E BPX Parameterisation repository](https://github.com/About-Energy-OpenSource/About-Energy-BPX-Parameterisation/). | |||
|
|||
To see how to use BPX with [PyBaMM](https://www.pybamm.org/), check out the [BPX example repository](https://github.com/pybamm-team/bpx-example). | |||
To see how to use BPX with [PyBaMM](https://www.pybamm.org/), check out the [BPX example repository](https://github.com/FaradayInstitution/bpx-example). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, now this link is broken (and a few other links to bpx-example
below), it should be in pybamm-team
repo.
@@ -9,7 +9,7 @@ import bpx | |||
filename = 'path/to/my/file.json' | |||
my_params = bpx.parse_bpx_file(filename) | |||
``` | |||
`my_params` will now be of type `BPX`, which acts like a python dataclass with the same attributes as the BPX format. To obatin example files, see the [A:E BPX Parameterisation repository](https://github.com/About-Energy-OpenSource/About-Energy-BPX-Parameterisation/) or [BPX example repository](https://github.com/pybamm-team/bpx-example). | |||
`my_params` will now be of type `BPX`, which acts like a python dataclass with the same attributes as the BPX format. To obatin example files, see the [A:E BPX Parameterisation repository](https://github.com/About-Energy-OpenSource/About-Energy-BPX-Parameterisation/) or [BPX example repository](https://github.com/FaradayInstitution/bpx-example). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bpx-example
repo is in pybamm-team
README.md
Outdated
@@ -26,7 +26,7 @@ import bpx | |||
filename = 'path/to/my/file.json' | |||
my_params = bpx.parse_bpx_file(filename) | |||
``` | |||
`my_params` will now be of type `BPX`, which acts like a python dataclass with the same attributes as the BPX format. To obatin example files, see the `examples` folder, the [A:E BPX Parameterisation repository](https://github.com/About-Energy-OpenSource/About-Energy-BPX-Parameterisation/), or the [BPX example repository](https://github.com/pybamm-team/bpx-example). | |||
`my_params` will now be of type `BPX`, which acts like a python dataclass with the same attributes as the BPX format. To obatin example files, see the `examples` folder, the [A:E BPX Parameterisation repository](https://github.com/About-Energy-OpenSource/About-Energy-BPX-Parameterisation/), or the [BPX example repository](https://github.com/FaradayInstitution/bpx-example). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And here as well, should be pybamm-team
.
No description provided.